Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace SIP003 variable in pluginArgs. #1974

Closed
wants to merge 5 commits into from
Closed

Replace SIP003 variable in pluginArgs. #1974

wants to merge 5 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Aug 18, 2018

Please follow the guide below

  • You will be asked some questions, please read them carefully and answer honestly

  • Put an x into all the boxes [ ] relevant to your pull request (like that [x])

  • Use Preview tab to see how your pull request will actually look like

  • Searched for similar pull requests

  • Compiled the code with Visual Studio

What is the purpose of your pull request?

  • Bug fix
  • Improvement
  • New feature

Description of your pull request and other information

Explanation of your pull request in arbitrary form goes here. Please make sure the description explains the purpose and effect of your pull request and is worded well enough to be understood. Provide as much context and examples as possible.

Replace %SS_REMOTE_HOST% .etc in pluginArgs to real value. So we can use something like -l %SS_LOCAL_HOST%:%SS_LOCAL_PORT% -r %SS_REMOTE_HOST%:%SS_REMOTE_HOST% as pluginArgs. Only cmd will parse environment variables in command line argument, API won't do that.

See #1818 and #1969

@ghost ghost changed the title When pluginOpts not exist, replace SIP003 variable in pluginArgs. Replace SIP003 variable in pluginArgs. Aug 19, 2018
@celeron533
Copy link
Contributor

Many thanks @studentmain .

refined the core logic as #1977

@ghost
Copy link
Author

ghost commented Aug 19, 2018

@celeron533 You can just close this one. If need, I'll open a PR for unit test.

@celeron533
Copy link
Contributor

Hi @studentmain , core logic has been merged in #1977.
Please send a new PR for unit test. Thx.

@celeron533 celeron533 closed this Aug 20, 2018
@ghost ghost mentioned this pull request Aug 20, 2018
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant